CUDA: support ncols > 1024 in bitonic argsort/top-k The original bito… - #26493
CUDA: support ncols > 1024 in bitonic argsort/top-k The original bito…#26493Geramy wants to merge 1 commit into
Conversation
…nic sort used when CUB is unavailable IE HIP, and some versions of older CUDA capped ncols to 1024 threads/blocks due to hardware LDS size limit and instead sent the request the the CPU. I have updated the kernel to now do multiple elements per thread ncols_pad/nthreads.
|
This would close issue #26399 (comment) |
|
This PR makes DeepSeek v4-Flash working fine on Strix Halo: there was a sudden drop of tg around 4k context (20->5 tok/s), which is not happening anymore with this PR merged. Thank you for work on this @Geramy 🙇 |
|
I tested this on NVIDIA by adding: in added in Added printing shared mem usage: and it seems to grow with the row length. Not sure how it works on AMD, but doesn't seem right. OK, before posting this I found your comment: So @Geramy is aware of this. But not sure what's the point of this PR then. Slightly increasing supported row length before switching to CPU? Ended up confused. What about kubakomu@42b262a, isn't this a more general AMD solution? |
Technically it should also be working on nvidia, but this isn't a nvidia problem because llama.cpp doesn't use this method for nvidia. I can guard it with a HIP condition and put the original back for the nvidia fallback. I do appreciate you checking on nvidia, I believe nvidia is 48KB LDS I can also adjust that, but it maxes out around 12884 elements I believe or close to it. |
|
probubly better to support the cub path on hip than trying to fit more into lds |
@IMbackK would you like me to close this PR and open one that implements CUB? I believe there are a few already but I can also do it too if it's preferred. |
|
@Geramy Since DeepSeek V4 has 1M max context length and So if we merged this PR as-is soon we would need another "CUDA: support ncols > 8192 in bitonic argsort/top-k" PR. I searched for hipCUB PRs but only found #26388 that is closed unmerged. I don't know what are the hipCUB argsort row length limits, I guess it's something that still needs to be checked. For me any solution is acceptable as long as it supports long row lengths, is reasonably fast and works correctly. hibCUB one seems easiest to implement. |
So technically instead of using |
|
I tried to use deepseek Flash v4 0731 since some days. PR 26388 had crashed too. Thanks for this, from my non programmer eye's this is the right way because it works. |
|
Interestingly with this PR ROCM did crash to for me right now again, on the strix halo. |
|
@Neresco are you building off of main? |
No i directly downloaded your fork and compiled it. |
|
Ok it crashes now at ~65k instead of 4k it looks like. |
Sure i am compiling and test. It will take ~45 minutes for the first result at ~65k threshold including compiling. |
Two times now my Window Manager crashed around 63k. |
we should move off of this PR and move into #26592 I will be closing this PR once the other one goes from DRAFT to not. |
|
PR #26592 moved out of draft closing this PR. |
Overview
This PR changes the argsort / top-k max cols to the maximum of the gpu available LDS by changing the kernel to use the maximum amount of X elements per thread. This was brought on due to the following issue:
#26399 (comment)
This mainly targets HIP devices due to there being no
ggml_cuda_op_top_kimplementation on hip.Additional information
Please note drift is not yet calculated in the below table.
Requirements